Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

revert(package): use exact uglify-es version (dependencies) (#199) #202

Merged
merged 3 commits into from
Jan 4, 2018
Merged

revert(package): use exact uglify-es version (dependencies) (#199) #202

merged 3 commits into from
Jan 4, 2018

Conversation

michael-ciniawsky
Copy link
Member

No description provided.

@michael-ciniawsky michael-ciniawsky added this to the 1.1.6 milestone Jan 3, 2018
@michael-ciniawsky
Copy link
Member Author

@kzc @filipesilva Does someone from the Uglify/Angular team know what exactly broke between uglify-es v3.2.0...3.3.0 ? Is this more of an uglify issue in general or specific to angular ?

@filipesilva
Copy link
Contributor

@michael-ciniawsky mishoo/UglifyJS#2663 was the problem affecting Angular projects, and it seems JQuery as well. It was fixed via mishoo/UglifyJS@cb62bd9 and released in 3.3.3 I believe.

@kzc
Copy link

kzc commented Jan 3, 2018

I can confirm what @filipesilva said. That bug has been fixed and released.

@filipesilva Could you test Angular against the latest uglify-es release to verify it's working correctly?

@michael-ciniawsky
Copy link
Member Author

Thx @filipesilva @kzc updated the minimum to v3.3.3, @filipesilva Is pinning the version of uglifyjs-webpack-plugin in angular-cli instead sufficient to avoid eventual breakage through an uglify update in uglifyjs-webpack-plugin or won't that work for the cli ?

kzc
kzc reviewed Jan 3, 2018
View reviewed changes
package.json Outdated
@@ -35,7 +38,7 @@
"serialize-javascript": "^1.4.0",
"schema-utils": "^0.3.0",
"source-map": "^0.6.1",
"uglify-es": "3.2.2",
"uglify-es": "^3.3.3",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason 3.3.3 was chosen?

     '3.2.2': '2017-12-10T18:36:46.421Z',
     '3.3.0': '2017-12-24T16:05:47.966Z',
     '3.3.1': '2017-12-25T13:11:01.187Z',
     '3.3.2': '2017-12-25T21:24:34.232Z',
     '3.3.3': '2017-12-28T21:32:12.606Z',
     '3.3.4': '2017-12-31T04:57:50.883Z' },

https://github.com/mishoo/UglifyJS2/commits/harmony

Copy link
Member Author

@michael-ciniawsky michael-ciniawsky Jan 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#202 (comment) is the only 'reason', I will update to v3.3.4 (@latest) :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@filipesilva
Copy link
Contributor

filipesilva commented Jan 4, 2018 •
edited
Loading

@michael-ciniawsky I can confirm that 3.3.3 and 3.3.4 do not show the problem affecting Angular applications.

Also I don't think this is intended, but you're also reverting the 1.1.5 release in this PR.

We can pin uglifyjs-webpack-plugin but I would prefer to keep it loose (as it is currently). Getting bugfixes from uglify-es is important for us.

@filipesilva filipesilva mentioned this pull request Jan 4, 2018
Open
alexander-akait
alexander-akait approved these changes Jan 4, 2018
View reviewed changes
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@michael-ciniawsky
Copy link
Member Author

Also I don't think this is intended, but you're also reverting the 1.1.5 release in this PR.

@filipesilva 😱 Thx 👍

@michael-ciniawsky michael-ciniawsky merged commit 426bafd into master Jan 4, 2018
@michael-ciniawsky michael-ciniawsky deleted the revert-199-master branch January 4, 2018 19:01
This was referenced Jan 5, 2018
Closed
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kzc kzc kzc left review comments

@alexander-akait alexander-akait alexander-akait approved these changes

@joshwiens joshwiens Awaiting requested review from joshwiens

Assignees
No one assigned
Labels
semver: Patch status: Approved type: Build
Projects
None yet
Milestone
1.1.6
Development

Successfully merging this pull request may close these issues.
5 participants
@michael-ciniawsky @filipesilva @kzc @alexander-akait @sokra